feat(sanic): Support span streaming#6319
Conversation
Codecov Results 📊✅ 67 passed | Total: 67 | Pass Rate: 100% | Execution Time: 6.14s 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ❌ Patch coverage is 9.52%. Project has 14506 uncovered lines. Files with missing lines (1)
Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 35.03% 35.68% +0.65%
==========================================
Files 190 190 —
Lines 22647 22553 -94
Branches 7722 8157 +435
==========================================
+ Hits 7934 8047 +113
- Misses 14713 14506 -207
- Partials 718 712 -6Generated by Codecov Action |
sl0thentr0py
force-pushed
the
neel/span-first/sanic
branch
from
d266281 to
950a311
Compare
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
sl0thentr0py
force-pushed
the
neel/span-first/sanic
branch
2 times, most recently
from
ca346f8 to
d10461b
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
sl0thentr0py
force-pushed
the
neel/span-first/sanic
branch
from
d10461b to
9e8d40b
Compare
sl0thentr0py
force-pushed
the
neel/span-first/sanic
branch
2 times, most recently
from
e9d3bf6 to
0f32453
Compare
ericapisani
left a comment
ericapisani
left a comment
There was a problem hiding this comment.
Some small things but otherwise LGTM. Approving so as not to block it from merging
sentrivana
left a comment
sentrivana
left a comment
There was a problem hiding this comment.
Looks good, couple small things _end -> end and TransactionSource -> SegmentSource
| with capture_internal_exceptions(): | ||
| scope = sentry_sdk.get_current_scope() | ||
| route_name = route.name.replace(request.app.name, "").strip(".") | ||
| scope.set_transaction_name(route_name, source=TransactionSource.COMPONENT) |
There was a problem hiding this comment.
There's a SegmentSource too for the streaming path. Would be good to use that if span streaming is enabled
There was a problem hiding this comment.
well the method itself is called set_transaction_name. If we want to clean this up properly, we should make a segment based method, just switching the source is half-assed
There was a problem hiding this comment.
I'll merge this for now, let's clean this up separately all together
| sanic_route = sanic_route[len(sanic_app_name) + 1 :] | ||
|
|
||
| scope.set_transaction_name( | ||
| sanic_route, source=TransactionSource.COMPONENT |
There was a problem hiding this comment.
Here too we should do SegmentSource if streaming
| ) | ||
| else: | ||
| scope.set_transaction_name( | ||
| rv[0].__name__, source=TransactionSource.COMPONENT |
sl0thentr0py
force-pushed
the
neel/span-first/sanic
branch
from
0f32453 to
6748b61
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6748b61. Configure here.
sl0thentr0py
force-pushed
the
neel/span-first/sanic
branch
from
7e2267f to
cb11ab0
Compare
sl0thentr0py
force-pushed
the
neel/span-first/sanic
branch
from
0a35b9d to
58850a3
Compare
4 participants
The
_unsampled_statusesoption is not respected for span streaming since we currently don't allow tail-based sampling in the new system.Issues